refactor(db): introduce ContextVar DAO layer and optimize transaction granularity#678
refactor(db): introduce ContextVar DAO layer and optimize transaction granularity#678yaojin3616 wants to merge 3 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a72456380
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| async with async_session() as session: | ||
| yield session |
There was a problem hiding this comment.
When no ContextVar transaction is active, this fallback opens a fresh AsyncSession and yields it, but the DAO write helpers only flush() and this context closes without committing. Several refactored callers still already have a request/session but are not wrapped in transaction() (for example BaseAuthProvider._create_new_user now calls find_or_create_identity(...) without passing its db), so first-time SSO/channel identity creation is rolled back before the subsequent User insert references it, leading to missing identities or FK failures. Bind the caller's session into the context or ensure standalone DAO write sessions commit/rollback.
Useful? React with 👍 / 👎.
| """ | ||
| # ── OrgMember binding ──────────────────────────────────────────────────── | ||
|
|
||
| async def bind_org_member(self, user: User) -> None: |
There was a problem hiding this comment.
Changing bind_org_member to accept only user leaves existing tenant join/create paths still calling registration_service.bind_org_member(db, new_user/current_user) in backend/app/api/tenants.py (checked lines 216, 230, 344, and 361). Those endpoints will now raise TypeError: ... takes 2 positional arguments but 3 were given after flushing user changes and before committing whenever a user creates or joins an organization/invitation, so update those call sites or retain a compatible signature.
Useful? React with 👍 / 👎.
1 participant
💡 Proposed Changes
This PR introduces two major database-related optimizations and a file encoding fix:
AsyncSessionseamlessly usingContextVar(_session_ctx), eliminating the need to explicitly passdb/sessionarguments through registry services, helpers, and handlers.verify_password_async,hash_password_async), external third-party network calls, Redis interactions, and read-only SELECT queries outsideasync with transaction()blocks.🧪 Verification
Verified that all 38 tests in the authentication, onboarding, registry, password reset, and SSO modules pass cleanly.